• Refactor anchor line scrolling to reapply focus until layout settles
• Extract line number parsing and focus logic into separate functions
• Add 10-second settle window with 250ms polling intervals
• Replace animation with direct scrollTop for better reliability
• Add regression tests for delayed content expansion scenarios

Refactor anchor scrolling with delayed settle mechanism

• Extract getHashedLineNumber() to parse and validate hash-based line numbers
• Create focusOnHashedLine() helper to handle scrolling and selection logic
• Refactor focusOnLine() to implement delayed settling mechanism with 250ms polling
• Replace jQuery animate with direct scrollTop() call for more reliable scrolling
• Add 10-second maximum settle duration to prevent indefinite polling

src/static/js/pad_editor.ts

Add Playwright test for delayed anchor scroll

• Add new Playwright-based test for anchor scroll regression
• Test that #L scroll reapplies when earlier content changes height
• Verify target line stays in viewport after dynamic content expansion
• Use polling to check scroll position and viewport offset stability

src/tests/frontend-new/specs/anchor_scroll.spec.ts

Add test for scroll reapplication on layout change

• Add regression test for scroll reapplication after content height changes
• Verify initial scroll position is maintained when earlier content expands
• Test that top offset increases appropriately when layout changes occur

src/tests/frontend/specs/scrollTo.js

Description

focusOnHashedLine() and the settle loop use $inner.find('div:nth-child(N)'), which can match
nested <div> elements inside earlier lines and scroll/select the wrong element. This breaks #L…
navigation for pads/plugins that introduce nested divs in line content.

Code

src/static/js/pad_editor.ts[R223-225]

+  const $inner = $aceOuter.contents().find('iframe').contents().find('#innerdocbody');
+  const line = $inner.find(`div:nth-child(${lineNumberInt})`);
+  if (line.length === 0) return false;

Evidence

#innerdocbody is the inner iframe body that Ace2Inner clears and then populates with line nodes as
direct children, so the intended target for “line N” is the Nth direct child line node. Using a
descendant selector (find) allows unrelated nested divs (including plugin-modified DOM) to match
earlier in document order, making the anchor scroll/selection nondeterministic when plugins add
block-level divs within a line.

src/static/js/pad_editor.ts[220-257]
src/static/js/pad_editor.ts[259-268]
src/static/js/ace2_inner.ts[3679-3694]
src/static/js/domline.ts[203-225]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The anchor scroll logic locates the target line via `$inner.find(`div:nth-child(${lineNumberInt})`)`, which searches all descendant divs and can select nested divs inside earlier lines. This can cause scrolling and `ace_setSelection()` to target the wrong node.
### Issue Context
In the inner iframe, `#innerdocbody` is cleared and then populated with the editor line nodes as direct children. Plugins can mutate each line node’s HTML (`acePostWriteDomLineHTML`), including inserting nested `<div>` elements.
### Fix Focus Areas
- src/static/js/pad_editor.ts[220-257]
- src/static/js/pad_editor.ts[259-268]
### Implementation notes
- Replace the descendant selector with a direct-child selection, for example:
- `const $line = $inner.children('div').eq(lineNumberInt - 1);`
- or `const $line = $inner.find(`> div:nth-child(${lineNumberInt})`);`
- Apply the same change in both `focusOnHashedLine()` and `getCurrentTargetOffset()` so the settle loop and the actual focus use the same element.
- Keep the existing `length === 0` guard and continue using the resolved node for `ace_setSelection()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new #L... behavior scrolls immediately on load via focusUntilStable() instead of deferring
until the pad layout has fully settled. This can still cause an initial incorrect viewport position
before late-loading content (for example images) finishes affecting layout.

Code

src/static/js/pad_editor.ts[R271-283]

+  const focusUntilStable = () => {
+    if (Date.now() - startTime >= maxSettleDuration) {
+      window.clearInterval(intervalId);
+      return;
}
-  }
+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+
+    focusOnHashedLine(ace, lineNumberInt);
+  };
+
+  focusUntilStable();
+  intervalId = window.setInterval(focusUntilStable, settleInterval);

Evidence

PR Compliance ID 1 requires anchor/line scrolling to occur only after content (including async
elements) has fully loaded and the final layout is stable. The new code calls focusUntilStable()
immediately (before any stability detection) and performs focusOnHashedLine() on each run, which
means scrolling happens prior to confirmed layout stabilization.

Defer anchor/line scrolling until pad content has fully loaded
src/static/js/pad_editor.ts[271-283]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The implementation scrolls immediately and then re-applies scrolling for a fixed settle window, but it does not actually *defer* scrolling until the layout has stabilized.
## Issue Context
Compliance requires that `#L...` navigation scroll occurs only after async content (e.g., images/plugins) has finished loading and the resulting layout is stable, to prevent landing on the wrong line/region.
## Fix Focus Areas
- src/static/js/pad_editor.ts[271-283]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

When loading a pad with a #L… hash, focusOnLine() re-runs focusOnHashedLine() every 250ms for up to
10s, and each run resets scroll position and Ace selection. This can prevent users from
scrolling/clicking away from the anchored line because their scroll/selection is continually
overwritten until the timer expires.

Code

src/static/js/pad_editor.ts[R266-283]

+  const maxSettleDuration = 10000;
+  const settleInterval = 250;
+  const startTime = Date.now();
+  let intervalId = null;
+
+  const focusUntilStable = () => {
+    if (Date.now() - startTime >= maxSettleDuration) {
+      window.clearInterval(intervalId);
+      return;
}
-  }
+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+
+    focusOnHashedLine(ace, lineNumberInt);
+  };
+
+  focusUntilStable();
+  intervalId = window.setInterval(focusUntilStable, settleInterval);

Evidence

padeditor.init() invokes exports.focusOnLine(self.ace) on load; focusOnLine() then starts a
setInterval for 10 seconds that calls focusOnHashedLine() every 250ms. focusOnHashedLine() sets
scrollTop and calls ace.ace_setSelection(), so each interval tick can override the user's current
scroll position and selection.

src/static/js/pad_editor.ts[39-55]
src/static/js/pad_editor.ts[213-285]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`exports.focusOnLine()` keeps forcing scroll + `ace_setSelection()` for the full settle window (10s) even after the target offset has stabilized, and even if the user interacts. This can make anchor links feel "stuck" on the target line.
## Issue Context
The intended behavior is to reapply the anchor scroll while late content changes layout. The current implementation does not detect stability or user intent; it simply runs until `maxSettleDuration` expires.
## Fix Focus Areas
- src/static/js/pad_editor.ts[255-285]
## Suggested fix approach
- Track the target line’s computed offset each tick (ideally the same offset basis used for scrolling).
- Clear the interval once the offset has remained within a small threshold for N consecutive ticks (e.g., 2–4).
- Additionally, cancel the interval on explicit user interaction events (wheel/touchmove/keydown/mousedown) so the code stops fighting the user.
- Ensure the existing regression tests still pass (late content insertion should still be corrected before the interval stops).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

focusOnLine() can stop reapplying after only ~750ms of no offset change (3 ticks * 250ms), so layout
shifts that occur later (e.g., images/plugins rendering after initial stabilization) will not be
corrected. This risks reintroducing the original “lands above intended line” behavior for
late-loading content.

Code

src/static/js/pad_editor.ts[R270-311]

+  // Settle window: keep correcting the scroll position while late content (images,
+  // plugin-rendered blocks) shifts the target line's offsetTop. The interval ends when
+  // either: (a) maxSettleDuration elapses, (b) the user interacts (see stop() below),
+  // or (c) the target offset has not changed for stableTicksRequired consecutive ticks
+  // (layout has settled — no need to keep re-scrolling).
+  const maxSettleDuration = 10000;
+  const settleInterval = 250;
+  const stableTicksRequired = 3;
+  const startTime = Date.now();
+  let intervalId: number | null = null;
+  let lastOffset: number | null = null;
+  let stableTicks = 0;
+
+  const userEventNames = ['wheel', 'touchmove', 'keydown', 'mousedown'];
+  const docs: Document[] = [];
+  const stop = () => {
+    if (intervalId != null) {
+      window.clearInterval(intervalId);
+      intervalId = null;
+    }
+    for (const doc of docs) {
+      for (const name of userEventNames) doc.removeEventListener(name, stop, true);
 }
+    docs.length = 0;
+  };
+
+  const focusUntilStable = () => {
+    if (Date.now() - startTime >= maxSettleDuration) {
+      stop();
+      return;
+    }
+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+    focusOnHashedLine(ace, lineNumberInt);
+    if (lastOffset != null && currentOffsetTop === lastOffset) {
+      stableTicks += 1;
+      if (stableTicks >= stableTicksRequired) stop();
+    } else {
+      stableTicks = 0;
+    }
+    lastOffset = currentOffsetTop;
+  };

Evidence

The reapply loop stops once the target offset is unchanged for 3 consecutive 250ms ticks (~750ms).
The linked bug report explicitly involves late-loading images shifting layout after the initial
scroll, which can happen after such a short stability window.

Delay scrolling to (scroll.js)
src/static/js/pad_editor.ts[270-311]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The anchor-scroll reapply loop can exit after ~750ms of stability, which can be earlier than late layout shifts (e.g., image load / plugin rendering) that the feature is meant to handle.
### Issue Context
Currently, the loop stops when `currentOffsetTop` is unchanged for `stableTicksRequired` consecutive ticks. With `settleInterval=250` and `stableTicksRequired=3`, the early-exit can happen in <1s.
### Fix Focus Areas
- src/static/js/pad_editor.ts[270-311]
### Suggested direction
- Introduce a **minimum settle window** (e.g., don’t allow early-exit until `Date.now() - startTime >= minSettleDurationMs`), or
- Increase the stability requirement to represent a meaningful time window (e.g., stable for >= 2s), or
- Use a hybrid: run for at least `minSettleDurationMs`, then allow early exit once stable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The stability detector uses strict equality on offset().top values, so small sub-pixel changes can
prevent stability from ever being detected. In that case, the loop will keep forcing
scroll/selection until the 10s timeout, increasing unnecessary work and extending the period where
programmatic scroll changes can be reverted.

Code

src/static/js/pad_editor.ts[R301-310]

+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+    focusOnHashedLine(ace, lineNumberInt);
+    if (lastOffset != null && currentOffsetTop === lastOffset) {
+      stableTicks += 1;
+      if (stableTicks >= stableTicksRequired) stop();
+    } else {
+      stableTicks = 0;
+    }
+    lastOffset = currentOffsetTop;

Evidence

The code compares currentOffsetTop === lastOffset and otherwise never increments stableTicks, so
any tiny oscillation prevents early exit and the interval runs until maxSettleDuration.

src/static/js/pad_editor.ts[296-311]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The loop’s stability check uses strict equality on pixel offsets, which is brittle and can prevent early-exit.
### Issue Context
`offset().top` is a number that can vary slightly (fractional pixels / rounding / tiny oscillations). Strict equality means the loop might never satisfy the “stable” condition.
### Fix Focus Areas
- src/static/js/pad_editor.ts[296-311]
### Suggested direction
- Replace strict equality with a tolerance, e.g.:
- `Math.abs(currentOffsetTop - lastOffset) < 1` (or another small epsilon), or
- compare rounded values (`Math.round(...)`) before checking stability.
- Consider updating `stableTicks` logic accordingly to avoid flapping.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The settle-window re-scrolling logic (timers/intervals) is non-obvious and is introduced without
explanatory comments about intent and tradeoffs. This increases maintenance risk and makes it harder
to reason about why these constants exist and when the interval should stop.

Code

src/static/js/pad_editor.ts[R266-280]

+  const maxSettleDuration = 10000;
+  const settleInterval = 250;
+  const startTime = Date.now();
+  let intervalId = null;
+
+  const focusUntilStable = () => {
+    if (Date.now() - startTime >= maxSettleDuration) {
+      window.clearInterval(intervalId);
+      return;
}
-  }
+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+
+    focusOnHashedLine(ace, lineNumberInt);
+  };

Evidence

PR Compliance ID 9 requires comments for complex/non-obvious logic. The added code introduces a
timed re-scroll loop (maxSettleDuration, settleInterval, setInterval) with implicit behavior
(always runs up to 10s unless timed out) but no comment explaining rationale or expected
stabilization conditions.

src/static/js/pad_editor.ts[266-280]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new settle-window interval logic is non-obvious and lacks comments explaining intent, stop conditions, and why the chosen constants are appropriate.
## Issue Context
Future changes to editor layout or plugin content could break this behavior, and reviewers/maintainers need clear documentation of the approach.
## Fix Focus Areas
- src/static/js/pad_editor.ts[266-280]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new Playwright anchor-scroll tests include a few short fixed waits (notably the 1s waits after
writeToPad and a 2s wait after navigation), which can be flaky on slow CI and unnecessarily slow on
fast CI. The longer waits (e.g. 3.5s) are explicitly testing the settle-window timing and are fine.

Code

src/tests/frontend-new/specs/anchor_scroll.spec.ts[R10-21]

+    await goToNewPad(page);
+    const padUrl = page.url();
+    await clearPadContent(page);
+    await writeToPad(page, Array.from({length: 30}, (_v, i) => `Line ${i + 1}`).join('\n'));
+    await page.waitForTimeout(1000);
+
+    await page.goto('about:blank');
+    await page.goto(`${padUrl}#L20`);
+    await page.waitForSelector('iframe[name="ace_outer"]');
+    await page.waitForSelector('#editorcontainer.initialized');
+    await page.waitForTimeout(2000);
+

Evidence

The tests use multiple waitForTimeout() calls even where a deterministic condition is available
(e.g., waiting for the expected line text / expected number of inner lines, or waiting until anchor
scrolling has actually occurred). This can lead to intermittent failures if the editor/content is
slower than the fixed delay.

src/tests/frontend-new/specs/anchor_scroll.spec.ts[9-21]
src/tests/frontend-new/specs/anchor_scroll.spec.ts[51-78]
src/tests/frontend-new/specs/anchor_scroll.spec.ts[80-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Some of the new Playwright tests rely on fixed sleeps for setup timing (especially after `writeToPad()` and after navigation). These can be flaky across CI environments.
### Issue Context
Some longer waits are intentional timing assertions (e.g., waiting past `minSettleDuration + stableTicksRequired*interval`). Those should remain. The short setup waits can be replaced with polling on observable editor state.
### Fix Focus Areas
- src/tests/frontend-new/specs/anchor_scroll.spec.ts[9-21]
- src/tests/frontend-new/specs/anchor_scroll.spec.ts[51-62]
- src/tests/frontend-new/specs/anchor_scroll.spec.ts[80-91]
### Suggested changes
- Replace `waitForTimeout(1000)` after `writeToPad()` with a wait for content to appear in the inner frame (e.g., poll until `#innerdocbody > div` count is >= 30 and/or line 20 contains expected text).
- Consider removing/reducing the `waitForTimeout(2000)` in the first test by waiting until the anchor scroll has actually happened (you already have `expect.poll(getScrollTop).toBeGreaterThan(10)`; that can serve as the synchronization point).
- Keep the timing-based waits that explicitly test early-exit/cancellation semantics.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

If the requested hashed line does not exist, getCurrentTargetOffset() returns null and the
interval keeps running until the 10s timeout. This repeatedly queries the DOM and keeps user-event
listeners registered even though the anchor can never be resolved.

Code

src/static/js/pad_editor.ts[R296-304]

+  const focusUntilStable = () => {
+    if (Date.now() - startTime >= maxSettleDuration) {
+      stop();
+      return;
+    }
+    const currentOffsetTop = getCurrentTargetOffset();
+    if (currentOffsetTop == null) return;
+    focusOnHashedLine(ace, lineNumberInt);
+    if (lastOffset != null && currentOffsetTop === lastOffset) {

Evidence

On each tick, the code returns early when the target line is missing and does not call stop(), so
the only termination path is the 10s maxSettleDuration guard.

src/static/js/pad_editor.ts[263-304]
src/static/js/pad_editor.ts[275-300]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When the hashed line is invalid/non-existent, the settle interval continues for up to 10 seconds doing repeated DOM queries.
### Issue Context
You likely still want retries briefly to handle cases where the inner doc is not yet rendered, but for truly invalid anchors it should bail out earlier.
### Fix Focus Areas
- src/static/js/pad_editor.ts[263-304]
- src/static/js/pad_editor.ts[275-300]
### Suggested direction
- Track consecutive “line missing” ticks and call `stop()` after a short threshold (e.g., 1–2 seconds) **or** after you can determine the document is fully present.
- Alternatively, stop once the line count is known and the requested line is definitively out of range.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools